Skip to content

9001 fix picking up from 7547 dumb http git repo support broken #9049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

9001 fix picking up from 7547 dumb http git repo support broken #9049

wants to merge 2 commits into from
+51 −24

Conversation

@costa
Copy link

@costa costa commented Nov 2, 2025

What was the end-user or developer problem that led to this PR?

See #9001

What is your fix for the problem, implemented in this PR?

Duplicate the functionality implemented in #7547

Make sure the following tasks are checked

p.s. Oh, and also containerised (with docker) test env was added.

hsbt
hsbt requested changes Nov 3, 2025
View reviewed changes
Copy link
Member

@hsbt hsbt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove Dockerfile and related changes? I don't use that.

If you interest that to use develop RubyGems, please consider to integrate the current devcontainer support of this repository.

@costa
Copy link
Author

costa commented Nov 7, 2025

@hsbt Dear Hiroshi, I'll make just this one attempt to convince you to keep the setup.
I'm using this standard of a simple dockerised bin/test in all my projects, it's dead-simple, explicit and straightforward way to run all the project's tests/specs — in a standard (Docker-based) env — and to have a seal on the code's ultimate correctness, CI/CD or not. I'm open to any suggestions, questions or comments, and I will remove that commit from this PR, of course, to get the rest of the changes merged, if you're unconvinced. Thank you for your work.

@colby-swandale
Copy link
Member

colby-swandale commented Nov 10, 2025

I agree with @hsbt that this is not something we're looking to adopt currently.

@costa
Copy link
Author

costa commented Nov 10, 2025

OK, @hsbt, please confirm, and I'll immediately remove these 39 lines from the 200K+(!) LoC project that might help casual contributors (who are definitely not "we") with a single entry point to running all the tests, having a single dependency (the ubiquitous Docker) at that. No problem.

@hsbt
Copy link
Member

hsbt commented Nov 11, 2025

Please remove them at first.

@costa costa force-pushed the 9001-fix-picking-up-from-7547-dumb-http-git-repo-support-broken branch from abd45e7 to 5510c62 Compare November 11, 2025 21:20
@costa
Copy link
Author

costa commented Nov 11, 2025

Please remove them at first.

@hsbt done

@costa
fixes ruby#9001 - dumb http git support for bundler update fixed...
9cf802a 
...by a little code duplication; specs are somewhat improved though
@costa costa force-pushed the 9001-fix-picking-up-from-7547-dumb-http-git-repo-support-broken branch from 5510c62 to 9cf802a Compare November 12, 2025 16:48
@costa
Merge remote-tracking branch 'upstream/master' into 9001-fix-picking-…
e85ad12 
…up-from-7547-dumb-http-git-repo-support-broken
@costa costa requested a review from hsbt November 15, 2025 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hsbt hsbt Awaiting requested review from hsbt

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@costa @colby-swandale @hsbt